Add configuration toggle for FlowExporter #5021
Merged
+381
−276
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yuntanghsu
force-pushed
the
flow_exporter_configuration
branch
3 times, most recently
from
13c006d
to
9ed313e
Compare
antoninbas
reviewed
May 24, 2023
There was a problem hiding this comment.
I would expect the documentation to be updated as well as part of this PR: docs/network-flow-visibility.md
I think that the commit message / PR description should include more information:
- non-backwards compatible changes to the Helm chart: FlowExporter-related values have all been moved under the
flowExporterdictionary - changes to the Antrea Agent config: FlowExporter-related fields are now grouped under the
flowExporterkey. Previous parameters are still supported but are considered deprecated, and will be removed in a future release.
| # If no PROTO is given, we consider "tls" as default. We support "tls", "tcp" and | ||
| # "udp" protocols. "tls" is used for securing communication between flow exporter and | ||
| # flow aggregator. | ||
| flowCollectorAddr: {{ .collectorAddr | quote }} |
There was a problem hiding this comment.
there is a small inconsistency between the Helm value path (flowExporter.collectorAddr) and the configuration parameter path (flowExporter. flowCollectorAddr). Should we rename the configuration parameter for consistency between the 2?
There was a problem hiding this comment.
Thanks for reviewing. I've renamed it to flowCollectorAddr.
network-flow-visibility.md is updated as well.
yuntanghsu
force-pushed
the
flow_exporter_configuration
branch
from
9ed313e
to
69a97bf
Compare
heanlan
previously approved these changes
May 26, 2023
yuntanghsu
force-pushed
the
flow_exporter_configuration
branch
from
69a97bf
to
6f2c3eb
Compare
antoninbas
reviewed
May 26, 2023
docs/network-flow-visibility.md
Outdated
Comment on lines
129
to
131
| #### Configuration pre Antrea v1.12.0 | ||
|
|
||
| Prior to the Antrea v1.12.0 release, the `flowExporter` option group in the |
There was a problem hiding this comment.
I think you mean v1.13?
docs/network-flow-visibility.md
Outdated
| Prior to the Antrea v1.12.0 release, the `flowExporter` option group in the | ||
| Antrea Agent configuration did not exist. To enable the Flow Exporter feature, | ||
| one simply needed to enable the feature gate, and the Flow Exporter related | ||
| configuration could be configured using (now deprecated) `flowCollectorAddr`, |
There was a problem hiding this comment.
s/(now deprecated)/the (now deprecated)
yuntanghsu
force-pushed
the
flow_exporter_configuration
branch
from
6f2c3eb
to
d96739d
Compare
… when flowAggregator is not installed 1. Add non-backwards compatible changes to the Helm chart: a. Change name flowCollector to flowExporter. b. FlowExporter-related values have all been moved under the flowExporter dictionary. 2. Add changes to the Antrea Agent config: FlowExporter-related fields are now grouped under the flowExporter key. Previous parameters are still supported but are considered deprecated, and will be removed in a future release. 3. Update network-flow-visibility.md Signed-off-by: Yun-Tang Hsu <[email protected]>
yuntanghsu
force-pushed
the
flow_exporter_configuration
branch
from
d96739d
to
4b23be3
Compare
|
/test-all |
1 similar comment
|
/test-all |
antoninbas
approved these changes
May 31, 2023
antoninbas
added
action/release-note
heanlan
added a commit
to heanlan/theia-1
that referenced
this pull request
Jun 2, 2023
A configuration toggle for flow exporter has recently been added to antrea agent config. We need to enable both the feature gate and the toggle to enable flow exporter feature. antrea-io/antrea#5021 Signed-off-by: heanlan <[email protected]>
heanlan
added a commit
to heanlan/theia-1
that referenced
this pull request
Jun 2, 2023
A configuration toggle for flow exporter has recently been added to antrea agent config. We need to enable both the feature gate and the toggle to enable flow exporter feature. antrea-io/antrea#5021 Signed-off-by: heanlan <[email protected]>
heanlan
added a commit
to antrea-io/theia
that referenced
this pull request
Jun 2, 2023
A configuration toggle for flow exporter has recently been added to antrea agent config. We need to enable both the feature gate and the toggle to enable flow exporter feature. antrea-io/antrea#5021 Signed-off-by: heanlan <[email protected]>
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adding configuration toggle for flow exporter to prevent antrea agent from showing error in log, which happens when user enable flow exporter feature gate without installing flow aggregator.
Fixes #4953
Signed-off-by: Yun-Tang Hsu [email protected]